Recursively compute variables #263
Conversation
|
I couldn't find a |
|
Note of course this doesn't deal with saving intermediates to disk (or checking for saved intermediates), which is also discussed in #3, but I think what's here is already a reasonable improvement. |
|
Wow, this is surprisingly elegant! The logic and tests look good to me. One big thing: have we tested this out within a Besides that, a couple documentation requests:
|
There was a problem hiding this comment.
Thanks @spencerahill! I agree this should be highlighted in the documentation. It will certainly simplify my object library quite a bit. I'll add a full docstring to recursively_compute_variable and an example to the official docs sometime later today or tomorrow.
| if isinstance(files_list, str): | ||
| files_list = [files_list] | ||
| subprocess.call(['dmget'] + files_list) | ||
| subprocess.call(['dmget'] + archive_files) |
There was a problem hiding this comment.
I was finding that the test suite would hang if dmget was called on an existing file that was not stored in /archive. I was able to reproduce this outside of aospy, so it has nothing to do with the changes I made to allow for the recursive calculation of variables; they must have made some updates to dmget. The logic here filters out files that aren't stored on archive before passing them to dmget.
aospy/data_loader.py
Outdated
| @@ -296,7 +296,7 @@ def recursively_compute_variable(self, var, start_date=None, end_date=None, | |||
| data = [self.recursively_compute_variable( | |||
| v, start_date, end_date, time_offset, **DataAttrs) | |||
| for v in var.variables] | |||
| return var.func(*data) | |||
| return var.func(*data).rename(var.name) | |||
There was a problem hiding this comment.
Indeed, good suggestion to add a test for this. One minor change and things work smoothly!
|
All sounds/looks good. Just ping me when you're ready for final review. |
- Add a full docstring for compute_variable_recursively - Add an example of this functionality to the official documentation - Make test more succinct in test_calc_basic
|
I guess I spoke too soon in #259; that's the failing test here :) @spencerahill anyway, whenever you get a chance, this should be ready for a final review! |
Bummer. Still, it seems like the frequency of the failures has gone down...maybe it's on its way toward going away completely. In it goes! Thanks @spencerkclark, this is great. |
Closes #3
This allows one to compose variable objects whose functions take other computed variables as arguments. For example: